🔴 new (require('./sheets'))() fails because sheets.js exports a singleton instance, not a class

core/sheets.js:74 exports module.exports = new SheetsService() — a singleton instance, not the SheetsService class. In core/leads-bridge.js:30, new (require('./sheets'))() attempts to call new on that instance, which will throw TypeError: (intermediate value) is not a constructor at runtime. The function syncLeadsToSheets() can never succeed.

Was this helpful? React with 👍 or 👎 to provide feedback.

🔴 Failed-to-sync leads are incorrectly marked as synced

At core/leads-bridge.js:58, all unsynced leads are unconditionally added to the synced list, even if sheetsService.addLead() threw an error for some of them (caught at line 52). This means leads that failed to sync to Google Sheets will never be retried on subsequent runs, since they'll already be in synced_leads.json. The code should only mark successfully synced leads.

In core/leads-bridge.js, lines 43-59: Track which leads were successfully synced in an array (e.g., `successfullySynced`), push each lead to it inside the try block after the synced++ increment (around line 51), and then on line 58, use that array instead of `unsynced` when building `newSynced`. This ensures only leads that were actually synced to Google Sheets get marked in synced_leads.json.

Was this helpful? React with 👍 or 👎 to provide feedback.

🔴 Apify dataset items API returns a JSON array, not a {data: {items: []}} object

The Apify API endpoint /v2/actor-runs/{runId}/dataset/items returns a raw JSON array of items, not an object with a data.items wrapper. At agents/apify_dubai_scraper.py:109-110, apify_request parses the response with json.loads() which yields a Python list. Then result.get("data", {}).get("items", []) will raise AttributeError: 'list' object has no attribute 'get' because lists don't have .get(). This means fetch_results() will always crash after a successful actor run.

Was this helpful? React with 👍 or 👎 to provide feedback.

🔴 Bayut URL construction double-encodes area: urllib.parse.quote() then .replace(' ', '-') is a no-op

At agents/dubai_villa_scraper.py:219-220, when area is provided, encoded_area = urllib.parse.quote(area) converts spaces to %20 (e.g., "Palm Jumeirah" → "Palm%20Jumeirah"). The subsequent .lower().replace(' ', '-') on line 220 is then a no-op since there are no literal spaces left, producing a URL like .../villa/palm%20jumeirah/ instead of the expected .../villa/palm-jumeirah/. This will likely return a 404 or wrong results from Bayut.

Was this helpful? React with 👍 or 👎 to provide feedback.

core/sheets.js exports an already-instantiated SheetsService (module.exports = new SheetsService()), but this code treats it like a constructor (new (require("./sheets"))()). That will throw at runtime. Use the exported instance directly and call init()/addLead() on it.

This marks all unsynced leads as synced even if some addLead calls fail (errors are caught but the lead is still appended to synced_leads.json). This will prevent retries and can permanently drop leads from being synced. Track only successfully synced leads (or update the synced file incrementally on each success) before writing synced_leads.json.

Bayut URL construction mixes quote() with a slug-style path. For areas like "Palm Jumeirah", urllib.parse.quote(area) yields Palm%20Jumeirah, so the URL becomes /villa/palm%20jumeirah/… rather than the expected hyphenated slug. Build the slug first (lowercase + replace spaces with '-') and then quote (or avoid quoting if the slug is URL-safe) to ensure the listings page resolves correctly.

const sheets = require("./sheets"); is unused (and syncLeadsToSheets requires the module again later). After fixing the SheetsService instantiation, keep a single import and use it to avoid duplication and dead code.

area_slug is computed but never used. Remove it or use it consistently when building the query/location parameter to keep the scraping logic clear.

json_matches is computed but never used. If the intent is to parse window.__INITIAL_STATE__ for structured listing data (more robust than regex), consider implementing that; otherwise remove this block to reduce dead code.

fetch_url uses the PEP 604 union type (str | None), which requires Python 3.10+. The repo README currently targets “system python” without a minimum version, and other agents avoid this syntax. Consider switching to Optional[str] (and similar) or explicitly documenting/enforcing Python >= 3.10 for agents.

deduplicate() keys off lead["phone"], but normalization here only strips spaces/dashes. The same UAE number can appear as 00971..., +971..., or local 05... and won’t dedupe correctly. Normalize to a single canonical format (e.g. E.164 +971...) before adding to normalized.

This file also uses PEP 604 union types in return annotations (e.g. str | None, dict | None), which require Python 3.10+. If agents are intended to run on an unspecified “system python”, consider using Optional[...]/Union[...] or documenting/enforcing Python >= 3.10.

fetch_results assumes apify_request returns an object with data.items, but the dataset items endpoint commonly returns a raw JSON array. If a list is returned, result.get(...) will throw and the scraper will crash. Adjust apify_request/fetch_results to handle a list response (or request a response shape that’s always an object).

⚠️ Potential issue | 🟠 Major

Load .env here or change the documented CLI contract.

The docstring says APIFY_TOKEN can live in .env, but this entry point only checks os.environ. Running python agents/apify_dubai_scraper.py as documented will still hit APIFY_TOKEN not set unless the caller exported the variable first.

Also applies to: 175-180

Verify each finding against the current code and only fix it if needed.

In `@agents/apify_dubai_scraper.py` around lines 7 - 12, The script documents that
APIFY_TOKEN can live in a .env but only reads os.environ; fix by loading .env at
module start (before any env access) using python-dotenv: add "from dotenv
import load_dotenv" and call "load_dotenv()" near the top of the file before
retrieving APIFY_TOKEN (and similarly before the env checks around the block
referenced at lines ~175-180). Alternatively, update the CLI docs to remove the
.env claim — but the preferred fix is to call load_dotenv() before accessing
APIFY_TOKEN so os.environ sees variables from .env.

⚠️ Potential issue | 🔴 Critical

# First, find and inspect the apify_dubai_scraper.py file
find . -name "apify_dubai_scraper.py" -type f

# Search for the apify_request function to understand response handling
rg "def apify_request" -A 10 -t py

# Inspect the actual fetch_results function in the file to confirm code and line numbers
cat -n agents/apify_dubai_scraper.py | sed -n '100,120p'

# Also check the full context around the function to understand usage
cat -n agents/apify_dubai_scraper.py | sed -n '1,30p'

# Check the complete apify_request function to see full implementation
cat -n agents/apify_dubai_scraper.py | sed -n '40,60p'

# Verify if there are any other usages of apify_request to understand the response pattern
rg "apify_request" agents/apify_dubai_scraper.py -B 1 -A 1

Treat the dataset-items response as a raw list.

Apify's /actor-runs/{runId}/dataset/items endpoint returns a JSON array directly (e.g. [{...},{...}]), not a wrapped object. The current code calls .get("data", {}) on the response, which will fail with AttributeError: 'list' object has no attribute 'get'. This differs from other Apify endpoints in this file (e.g. /actor-runs/{run_id}) which return wrapped responses.

 def fetch_results(token: str, run_id: str) -> list[dict]:
     """Fetch results from completed actor run."""
-    result = apify_request("GET", f"/actor-runs/{run_id}/dataset/items", token)
-    items = result.get("data", {}).get("items", [])
+    items = apify_request("GET", f"/actor-runs/{run_id}/dataset/items", token)
+    if not isinstance(items, list):
+        log(f"Unexpected dataset response: {items}")
+        return []
     log(f"Fetched {len(items)} items from Apify")
     return items

Verify each finding against the current code and only fix it if needed.

In `@agents/apify_dubai_scraper.py` around lines 107 - 111, The fetch_results
function assumes apify_request returned a dict and calls .get on it, but the
/actor-runs/{run_id}/dataset/items endpoint returns a raw list; update
fetch_results to handle both shapes by checking the type of the response from
apify_request (called in fetch_results) and set items = result if it's a list,
otherwise fall back to result.get("data", {}).get("items", []); keep the
log(f"Fetched {len(items)} items from Apify") and ensure the function returns
the items list.

⚠️ Potential issue | 🟠 Major

Serialize writes to the shared leads store.

This code loads data/state/villa_leads.json, appends in memory, and rewrites the whole file. agents/dubai_villa_scraper.py does the same against the same path, so overlapping runs will drop whichever scraper saves first.

Also applies to: 198-201

Verify each finding against the current code and only fix it if needed.

In `@agents/apify_dubai_scraper.py` around lines 148 - 158, Concurrent runs
overwrite the shared JSON leads store because load_existing_leads and save_leads
read the whole file, mutate in-memory, and rewrite it; change these functions to
perform concurrency-safe updates (e.g., acquire a file lock around
read-modify-write or switch to an append-only/JSONL writer) so overlapping
scrapers don't lose data. Specifically, update load_existing_leads, save_leads
and any callers that append to LEADS_FILE so they obtain an exclusive lock on
LEADS_FILE (or open it in append mode for JSONL) before reading/writing, merge
new leads into the existing set safely, and use atomic replace/rename when
writing to avoid partial writes; ensure the same locking/format is used by
agents/dubai_villa_scraper.py to keep behavior consistent.

⚠️ Potential issue | 🟠 Major

The shared leads store still has a lost-update race.

Like the Apify agent, this flow rewrites data/state/villa_leads.json via a read-modify-write sequence with no locking. If both agents overlap, the later write drops the earlier agent’s newly found leads.

Also applies to: 302-319

Verify each finding against the current code and only fix it if needed.

In `@agents/dubai_villa_scraper.py` around lines 49 - 59, The read-modify-write on
LEADS_FILE in load_existing_leads/save_leads causes lost-update races; wrap the
entire sequence that reads, merges new leads, and writes back with a file-level
lock (e.g., using a FileLock/portalocker) so concurrent agents serialize access
to LEADS_FILE, perform a read->merge-by-unique-id->write under the same lock,
and write atomically (write to temp + os.replace) to avoid corruption; apply the
same locking/merge pattern to the other lead-write logic referenced around the
second block (lines 302-319) and reference the functions load_existing_leads and
save_leads when locating the code to change.

⚠️ Potential issue | 🟠 Major

Paginate Bayut results before honoring --max.

This implementation requests exactly one Bayut search page and never advances a page parameter. Any run that needs more leads than that first page exposes will silently under-collect, even though --max suggests otherwise.

Verify each finding against the current code and only fix it if needed.

In `@agents/dubai_villa_scraper.py` around lines 219 - 237, The scraper only
fetches the first Bayut search page so it under-collects when max_results is
larger; modify the listing collection around
encoded_area/url/fetch_url/listing_urls to paginate (add a page parameter, e.g.
append "&page={page}" to the existing query), loop fetching pages starting at
page=1, extract and accumulate unique listing URLs into listing_urls until you
have >= max_results or no new URLs are returned, and then slice to max_results
before iterating to create leads; ensure you break on empty pages and update
logs to show pages fetched.

⚠️ Potential issue | 🔴 Critical

Use the exported Sheets singleton instead of constructing it.

core/sheets.js already exports a service instance. Calling new (require("./sheets"))() here will throw before any sync happens; reuse the imported sheets object or export the class separately.

 async function syncLeadsToSheets() {
-  const sheetsService = new (require("./sheets"))();
+  const sheetsService = sheets;
   const initialized = await sheetsService.init();

Verify each finding against the current code and only fix it if needed.

In `@core/leads-bridge.js` around lines 29 - 31, The syncLeadsToSheets function
instantiates the Sheets module with new (require("./sheets"))() which is
incorrect because core/sheets.js exports a singleton; change the code to
require/import the exported sheets instance (the exported symbol from
"./sheets") and use that instance (e.g., sheets.init()) instead of creating a
new object; if you actually need the class, update core/sheets.js to also export
the class constructor and require that specific export, but by default reuse the
exported sheets singleton rather than calling new on require("./sheets").

⚠️ Potential issue | 🔴 Critical

Only persist leads that actually synced.

saveSynced() currently appends every unsynced lead, even when addLead() threw. A transient Sheets failure will therefore permanently suppress retries for that lead.

   let synced = 0;
+  const syncedNow = [];
   for (const lead of unsynced) {
     try {
       await sheetsService.addLead({
         name: lead.name,
         phone: lead.phone,
         notes: `Area: ${lead.area} | Price: ${lead.price} | Source: ${lead.source} | ${lead.url}`
       });
       synced++;
+      syncedNow.push(lead);
     } catch (err) {
       console.error(`[leads-bridge] Failed to sync ${lead.phone}:`, err.message);
     }
   }
 
   // Mark all as synced
-  const newSynced = [...loadSynced(), ...unsynced];
+  const newSynced = [...loadSynced(), ...syncedNow];
   saveSynced(newSynced);

Verify each finding against the current code and only fix it if needed.

In `@core/leads-bridge.js` around lines 43 - 59, The code currently appends all
unsynced leads to persistent storage regardless of whether sheetsService.addLead
succeeded; change the logic to only persist leads that were actually synced by
collecting successful leads (e.g., push to a local array inside the try block or
incrementally build syncedLeads) and then call saveSynced([...loadSynced(),
...syncedLeads]) instead of saveSynced([...loadSynced(), ...unsynced]); keep the
existing try/catch around sheetsService.addLead and error logging for failed
attempts so transient failures remain retryable.

⚠️ Potential issue | 🟠 Major

# First, let's find the agents directory and list its contents
find . -name "agents" -type d | head -20
git ls-files | grep -E "agents/" | head -20

# Check if the agent files exist
ls -la agents/ 2>/dev/null || echo "agents/ directory not found at root"

# Search for these specific files
find . -name "dubai_villa_scraper.py" -o -name "apify_dubai_scraper.py"

# Check for version requirements in common config files
cat pyproject.toml 2>/dev/null | grep -i python || echo "No python requirement in pyproject.toml"
cat setup.py 2>/dev/null | grep -i python || echo "No setup.py found"
cat setup.cfg 2>/dev/null | grep -i python || echo "No setup.cfg found"

# Check the agent files for parameterized built-ins and union syntax
wc -l agents/dubai_villa_scraper.py agents/apify_dubai_scraper.py

# Search for parameterized built-ins (list[...], dict[...], etc.) and union syntax (X | Y)
rg "list\[|dict\[|tuple\[|set\[|\s\|\s" agents/

# Check the first 50 lines of each agent file for imports and type hints
head -50 agents/dubai_villa_scraper.py

# Check the apify agent file as well
head -50 agents/apify_dubai_scraper.py

Specify Python 3.10+ requirement for these commands.

The agents use PEP 604 union syntax (str | None) and parameterized built-ins (list[str], dict, etc.), which require Python 3.10+ and 3.9+ respectively. The README shows unqualified python commands, so users with system Python < 3.10 will fail at runtime. Add a note requiring Python 3.10+.

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 57 - 70, Update the README to explicitly require
Python 3.10+ for running the scraper commands: add a short note above the
examples stating "Requires Python 3.10+" (or "Python 3.10 or later") because the
agents (agents/dubai_villa_scraper.py and agents/apify_dubai_scraper.py) use PEP
604 union syntax (e.g., str | None) and newer parameterized built-ins; also keep
the APIFY_TOKEN note for the Apify scraper so users know environment setup
requirements.